Tabular UI for naming linked files #12624

priyanshu16095 · 2025-03-05T15:11:31Z

Closes #11368

This PR adds a tabular construct similar to "Key patterns" for "Linked files name" in the Linked files tab in preferences.

Mandatory checks

I own the copyright of the code submitted and I licence it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

src/main/java/org/jabref/gui/linkedfile/LinkedFileNamePatternsPanel.java

src/main/java/org/jabref/logic/citationkeypattern/CitationKeyGenerator.java

src/main/java/org/jabref/logic/citationkeypattern/Pattern.java

src/main/java/org/jabref/logic/linkedfile/AbstractLinkedFileNamePatterns.java

src/main/java/org/jabref/logic/linkedfile/GlobalLinkedFileNamePatterns.java

…emModel to PatternsItemModel

…ggestionCell to PatternsSuggestionCell

trag-bot · 2025-03-21T02:35:03Z

src/main/java/org/jabref/gui/linkedfile/LinkedFileNamePatternsPanelViewModel.java

+        if ((initialNamePattern.getDefaultValue() == null) || initialNamePattern.getDefaultValue().equals(Pattern.NULL_PATTERN)) {
+            defaultPattern = "";
+        } else {
+            defaultPattern = initialNamePattern.getDefaultValue().stringRepresentation();
+        }


The code violates the fail-fast principle by nesting logic inside else branch. Should check for valid state first and return/assign early.

Maybe it is OK in this context.

Is the comment OK? or the code?

trag-bot · 2025-03-21T02:36:18Z

src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTab.java

+    @Override
+    public void setValues() {
+        viewModel.setValues();
+        BibEntryTypesManager entryTypesManager = Injector.instantiateModelOrService(BibEntryTypesManager.class);


Direct instantiation through Injector in method body violates separation of concerns. The dependency should be injected through constructor or setter method.

Yes. Maybe you can add an argument BibEntryTypesManager to the constructor of LinkedFilesTab?

trag-bot · 2025-03-21T02:38:22Z

src/main/java/org/jabref/logic/bibtex/comparator/MetaDataDiff.java

@@ -48,7 +48,7 @@ public static Optional<MetaDataDiff> compare(MetaData originalMetaData, MetaData
            return Optional.empty();
        } else {
            MetaDataDiff diff = new MetaDataDiff(originalMetaData, newMetaData);
-            List<Difference> differences = diff.getDifferences(new GlobalCitationKeyPatterns(CitationKeyPattern.NULL_CITATION_KEY_PATTERN));
+            List<Difference> differences = diff.getDifferences(new GlobalCitationKeyPatterns(Pattern.NULL_PATTERN));


Same GlobalCitationKeyPatterns object is created twice in the class - in compare() and toString(). This violates DRY principle and creates redundant object allocations.

trag-bot · 2025-03-21T02:38:58Z

src/main/java/org/jabref/logic/citationkeypattern/AbstractCitationKeyPatterns.java

@@ -14,12 +14,12 @@
 */
 public abstract class AbstractCitationKeyPatterns {

-    protected CitationKeyPattern defaultPattern = CitationKeyPattern.NULL_CITATION_KEY_PATTERN;
+    protected Pattern defaultPattern = Pattern.NULL_PATTERN;


Public methods should not return null values. Consider using Optional instead of allowing null patterns, even for internal representation.

trag-bot · 2025-03-21T02:42:40Z

src/main/java/org/jabref/logic/citationkeypattern/Pattern.java

+    public static List<Pattern> getAllPatterns() {
+        return Arrays.stream(Pattern.class.getDeclaredFields())


The method lacks JavaDoc despite being a complex public method that uses reflection and handles exceptions. Documentation would help explain its purpose and behavior.

trag-bot · 2025-03-21T02:44:34Z

src/main/java/org/jabref/logic/linkedfile/AbstractLinkedFileNamePatterns.java

+    }
+
+    /**
+     * Gets an object for a desired key from this LinkedFileNamePattern or one of it's parents (in the case of


JavaDoc contains grammatical error ('it's' should be 'its') and references Hashtable in description while code uses HashMap, making documentation misleading.

trag-bot · 2025-03-21T02:45:02Z

src/main/java/org/jabref/logic/linkedfile/GlobalLinkedFileNamePatterns.java

+    }
+
+    public static GlobalLinkedFileNamePatterns fromPattern(String pattern) {
+        return new GlobalLinkedFileNamePatterns(new Pattern(pattern));


Missing validation for the pattern parameter before creating new Pattern object. Input parameters should be validated to fail fast.

trag-bot · 2025-03-21T02:47:06Z

src/main/java/org/jabref/migrations/PreferencesMigrations.java

+        if (preferenceFileNamePattern != null) {
+            mainPrefsNode.put(JabRefCliPreferences.LINKED_FILE_NAME_PATTERNS_NODE, "[auth_year]");
+            if (prefs.hasKey(JabRefCliPreferences.IMPORT_FILENAMEPATTERN)) {


The code violates the fail-fast principle by nesting logic inside conditional blocks instead of returning early. The nested if condition should be restructured.

trag-bot · 2025-03-21T02:47:08Z

src/main/java/org/jabref/migrations/PreferencesMigrations.java

@@ -68,6 +68,7 @@ public static void runMigrations(JabRefGuiPreferences preferences) {
        upgradeCleanups(preferences);
        moveApiKeysToKeyring(preferences);
        removeCommentsFromCustomEditorTabs(preferences);
+        upgradeFileImportPatternToLinkedFileNamePattern(preferences, mainPrefsNode);


The method lacks proper JavaDoc documentation explaining the migration process, parameters, and potential side effects, which is important for complex migration operations.

priyanshu16095 · 2025-03-21T04:07:25Z

To remove code duplication ,
Renamed LinkedFileNamePattern and CitationKeyPattern to Pattern
Renamed LinkedFileNamePatternsPanelItemModel and CitationKeyPatternsItemModel to PatternsItemModel
Renamed LinkedFileNamePatternsSuggestionCell and CitationKeyPatternsSuggestionCell to PatternsSuggestionCell

The thing I am not sure about is whether the preference migration is correct or not.

InAnYan · 2025-03-25T16:51:00Z

src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTab.fxml

-        <TextField fx:id="fileDirectoryPattern" GridPane.columnIndex="1" GridPane.rowIndex="1"
-                   prefWidth="300" minWidth="300" maxWidth="300"/>
-    </GridPane>
+    <Label text="%( Note: Press return to commit changes in the table! )"/>


Maybe it's better to remove parenthesis (to ease translation).

And change text style to italics

I used this because it is similarly used in the citation key pattern naming table.

To maintain a consistent UI across JabRef, I used it here. Should I change it?

InAnYan · 2025-03-25T16:52:51Z

src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java

+                new GlobalLinkedFileNamePatterns(filePreferences.fileNamePatternProperty().get().getDefaultValue());
+        patternListProperty.forEach(item -> {
+            String patternString = item.getPattern();
+            if (!"default".equals(item.getEntryType().getName())) {


Magic value?

Or IntelliJ IDEA hint to prevent operations on null values?

InAnYan · 2025-03-25T16:53:52Z

src/main/java/org/jabref/gui/preferences/linkedfiles/LinkedFilesTabViewModel.java

    private final StringProperty fileDirectoryPatternProperty = new SimpleStringProperty();
    private final BooleanProperty confirmLinkedFileDeleteProperty = new SimpleBooleanProperty();
    private final BooleanProperty moveToTrashProperty = new SimpleBooleanProperty();

+    private final ListProperty<PatternsItemModel> patternListProperty = new SimpleListProperty<>(FXCollections.observableArrayList());
+    private final ObjectProperty<PatternsItemModel> defaultNamePatternProperty = new SimpleObjectProperty<>(
+            new PatternsItemModel(new LinkedFileNamePatternsPanelViewModel.DefaultEntryType(), ""));


I saw you used empty strings ("") in some places.

Does it differ somewhat from Pattern.NULL_PATTERN?

trag-bot · 2025-03-26T14:54:41Z

src/main/java/org/jabref/logic/importer/util/MetaDataParser.java

@@ -103,8 +102,8 @@ public MetaData parse(Map<String, String> data, Character keywordSeparator) thro
     * @return the given metaData instance (which is modified, too)
     */
    public MetaData parse(MetaData metaData, Map<String, String> data, Character keywordSeparator) throws ParseException {
-        CitationKeyPattern defaultCiteKeyPattern = CitationKeyPattern.NULL_CITATION_KEY_PATTERN;
-        Map<EntryType, CitationKeyPattern> nonDefaultCiteKeyPatterns = new HashMap<>();
+        Pattern defaultCiteKeyPattern = org.jabref.logic.citationkeypattern.Pattern.NULL_PATTERN;


The refactoring from CitationKeyPattern to Pattern should be accompanied by updated tests to ensure the new implementation behaves as expected.

jabref-machine · 2025-03-26T14:55:13Z

JUnit tests are failing. In the area "Some checks were not successful", locate "Tests / Unit tests (pull_request)" and click on "Details". This brings you to the test output.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

jabref-machine · 2025-03-26T14:55:13Z

Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of [x] (done), [ ] (not done yet) or [/] (not applicable).

priyanshu16095 added 26 commits February 13, 2025 03:42

Add fix options for integrity issues

9b11edf

Fix the check

02d8e75

Add a missing entry for Localization.lang

754b6ad

Fix the check

696f904

Refactor code

4ab36a8

Fix the check

28de796

Add a CHANGELOG.md entry

70ae8eb

Refactor the code

77df6ea

Make use of IntegrityIssue enum

30c81ce

Add keys in Localization.lang

5b389da

Refactor the code

3a0d9a1

Remove use of enum

a04e06a

Remove use of enum from PageChecker class

8e9341a

Add fixAll method

235c74f

Add a check for fixAll and fixByType method

9839137

Fix a bug

5754680

Use dialogService.notify() to notify

8f7bdc2

Remove unused imports

f57719c

Merge branch 'main' into integrityCheckActions

2105a13

Merge branch 'main' into integrityCheckActions

e37ba79

Add tests for methods

139777e

Fix the check and updates based on review

39dbfd1

Use enum for passing issue message

16e9817

Fix the checkstyle check

102e789

Add the gui

92d91b5

Add files in logic directory

b726f30

priyanshu16095 changed the title ~~Tabular UI~~ Tabular UI for naming linked files Mar 5, 2025

priyanshu16095 added 3 commits March 5, 2025 20:59

Revert changes in three files

86518fc

Revert some more files

6f985ec

Revert some more files

5ea4754

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/gui/linkedfile/LinkedFileNamePatternsPanel.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/citationkeypattern/CitationKeyGenerator.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/citationkeypattern/Pattern.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/linkedfile/AbstractLinkedFileNamePatterns.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/linkedfile/AbstractLinkedFileNamePatterns.java Show resolved Hide resolved

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

src/main/java/org/jabref/logic/linkedfile/GlobalLinkedFileNamePatterns.java Show resolved Hide resolved

priyanshu16095 added 5 commits March 21, 2025 07:03

Renmae LinkedFileNamePatternsPanelItemModel and CitationKeyPatternsIt…

a3383a2

…emModel to PatternsItemModel

Rename LinkedFileNamePatternsSuggestionCell and CitationKeyPatternsSu…

967a4a5

…ggestionCell to PatternsSuggestionCell

Fix the checkstyle check

1261731

Remove the redundant check

965d4df

Fix the check

a0a00ef

priyanshu16095 marked this pull request as ready for review March 21, 2025 02:29

trag-bot bot reviewed Mar 21, 2025

View reviewed changes

InAnYan reviewed Mar 25, 2025

View reviewed changes

priyanshu16095 marked this pull request as draft March 26, 2025 13:37

Fix import in MetaDataParse.java

ba11cf3

trag-bot bot reviewed Mar 26, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitHub Sponsors

Tabular UI for naming linked files #12624

Tabular UI for naming linked files #12624

priyanshu16095 commented Mar 5, 2025 •

edited

Loading

trag-bot bot Mar 21, 2025

koppor Mar 21, 2025

priyanshu16095 Mar 26, 2025

trag-bot bot Mar 21, 2025

InAnYan Mar 25, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

trag-bot bot Mar 21, 2025

priyanshu16095 commented Mar 21, 2025

InAnYan Mar 25, 2025 •

edited

Loading

priyanshu16095 Mar 25, 2025

priyanshu16095 Mar 26, 2025

InAnYan Mar 25, 2025

InAnYan Mar 25, 2025

trag-bot bot Mar 26, 2025

jabref-machine commented Mar 26, 2025

jabref-machine commented Mar 26, 2025

		public static List<Pattern> getAllPatterns() {
		return Arrays.stream(Pattern.class.getDeclaredFields())

Tabular UI for naming linked files #12624

Are you sure you want to change the base?

Tabular UI for naming linked files #12624

Conversation

priyanshu16095 commented Mar 5, 2025 • edited Loading

Closes #11368

Mandatory checks

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

koppor Mar 21, 2025

Choose a reason for hiding this comment

priyanshu16095 Mar 26, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

InAnYan Mar 25, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 21, 2025

Choose a reason for hiding this comment

priyanshu16095 commented Mar 21, 2025

InAnYan Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

priyanshu16095 Mar 25, 2025

Choose a reason for hiding this comment

priyanshu16095 Mar 26, 2025

Choose a reason for hiding this comment

InAnYan Mar 25, 2025

Choose a reason for hiding this comment

InAnYan Mar 25, 2025

Choose a reason for hiding this comment

trag-bot bot Mar 26, 2025

Choose a reason for hiding this comment

jabref-machine commented Mar 26, 2025

jabref-machine commented Mar 26, 2025

priyanshu16095 commented Mar 5, 2025 •

edited

Loading

InAnYan Mar 25, 2025 •

edited

Loading